feat: signup form functionality update #19
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the signup form by moving authentication logic from the parent page component into the SignupForm component itself. This change makes the SignupForm self-contained and directly handles user registration and navigation.
Key changes:
- Moved
authClient.signUp.email()logic from the page component into SignupForm'sonSubmithandler - Removed the
handleSignupprop from SignupForm interface, making it self-sufficient - Added router-based navigation on successful signup
- Removed the 'use client' directive from the page component (no longer needed since it doesn't use client-side hooks)
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/client/src/components/sections/authentications/default/SignupForm.tsx | Internalized signup logic, added router navigation, reorganized imports, and updated Button props formatting |
| apps/client/src/app/auth/signup/page.tsx | Simplified to a presentational component by removing client-side logic and the handleSignup function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (signupData) { | ||
| router.push(rootPaths.root); | ||
| } | ||
| if (error) { | ||
| setError('root.credential', { type: 'manual', message: error.message }); |
There was a problem hiding this comment.
Both success and error conditions can be true simultaneously if signupData exists but error is also present. These should be mutually exclusive using else-if logic to prevent navigating away while also setting an error.
| if (signupData) { | |
| router.push(rootPaths.root); | |
| } | |
| if (error) { | |
| setError('root.credential', { type: 'manual', message: error.message }); | |
| if (error) { | |
| setError('root.credential', { type: 'manual', message: error.message }); | |
| } else if (signupData) { | |
| router.push(rootPaths.root); |
| router.push(rootPaths.root); | ||
| } | ||
| if (error) { | ||
| setError('root.credential', { type: 'manual', message: error.message }); |
There was a problem hiding this comment.
Potential null reference error if error.message is undefined. The error object should be checked for the existence of the message property before accessing it, or provide a fallback error message.
| setError('root.credential', { type: 'manual', message: error.message }); | |
| setError('root.credential', { type: 'manual', message: error?.message ?? "An unknown error occurred." }); |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Changes
How to test
Screenshots (if UI)
Checklist